fix(kiro): run kiro-cli through a PTY so usage loads (fixes #1619)#1744
Conversation
kiro-cli 2.8.1 (an Amazon Q Developer CLI fork) performs terminal setup on startup and emits no output at all when launched without a controlling terminal. The probe launched `whoami`, `chat --no-interactive /usage`, and `/context` over plain pipes, so every command hung until the 20s deadline and surfaced as "Kiro CLI timed out." — even though the same commands return in ~2-4s from a real terminal. Route every kiro-cli invocation through TTYCommandRunner (PTY), matching the Codex/Claude status probes. The existing parser handles the PTY output unchanged. The now-orphaned `isUsageOutputComplete` helper is removed; the pipe-based `runCommand` is left in place (still covered by its own tests). steipete#1627 only made /usage independent of the slow account probe and was never validated against real kiro-cli, so the timeout persisted on 2.8.1. Fixes steipete#1619. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 6:48 PM ET / 22:48 UTC. Summary Reproducibility: yes. at source/report level: the linked report gives a Kiro 2.8.1 timeout path, and current main still uses the pipe-backed Kiro command path. I did not run a live provider probe because AGENTS.md requires explicit request for that. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the PTY-based Kiro fix after latest checks complete and a maintainer accepts the shared PTY/process cleanup behavior for provider probes. Do we have a high-confidence way to reproduce the issue? Yes at source/report level: the linked report gives a Kiro 2.8.1 timeout path, and current main still uses the pipe-backed Kiro command path. I did not run a live provider probe because AGENTS.md requires explicit request for that. Is this the best way to solve the issue? Yes, PTY transport is the narrowest maintainable shape for a CLI that emits output only with a terminal, and current head now preserves exit status, idle-output checks, cancellation, and detached-helper cleanup. The remaining work is maintainer acceptance and completed checks, not a new code finding. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e4e97f9bc44c. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ac29c3751
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
clawsweeper
Bot
added
proof: sufficient
The PTY runner cannot surface the child exit status, so a non-zero `kiro-cli whoami` that prints a non-login error (config/network) would be parsed as a logged-in account with empty fields. Reject output carrying no account markers so the best-effort account probe reports it as unavailable. Addresses Codex review feedback on steipete#1744. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9424144fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
rating: 🐚 platinum hermit
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40140a3181
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
clawsweeper
Bot
added
rating: 🦐 gold shrimp
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
status: 👀 ready for maintainer look
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Landed in Proof: exact-head CI run No additional credentialed Kiro probe was run; the merge relies on the PR's existing real Kiro 2.8.1 and packaged-app proof. |
2 participants
Summary
Kiro usage timed out on
kiro-cli2.8.1 even though the same commands returned in seconds from a terminal. Fixes #1619.kiro-cliperforms terminal setup on startup and emits no output when launched over bare pipes. Routewhoami,/usage, and/contextthrough the shared PTY runner so GUI and packaged-app probes behave like terminal invocations.Final behavior
/usageoutput only after complete usage markers have arrived; deadlines and incomplete output still fail.Verification
swift test --filter KiroStatusProbeTests— 42 passedswift test --filter TTYCommandRunnerEnvTests— 26 passedswift test --filter SpawnedProcessGroupTests— 12 passedmake check— cleanmake test— 43/43 shards passedkiro-cli2.8.1 and packaged-app proof from the contributor: usage returned in seconds with no residual Kiro processes, and the packaged app populated Kiro usage instead of timing out.